Conversation
| self._max_n_locator = mticker.MaxNLocator(max_n_ticks, integer=True) | ||
| self._max_n_locator_days = mticker.MaxNLocator( | ||
| max_n_ticks, integer=True, steps=[1, 2, 4, 7, 14]) | ||
| max_n_ticks, integer=True, steps=[1, 2, 4, 7, 10]) |
There was a problem hiding this comment.
The docs mention that this should be between 1 and 10 but I'm not sure that is the ideal steps for nc-time-axis.
There was a problem hiding this comment.
Holy 💩 ... I've just joined the dots here!
During the marathon that was SciTools/iris#3019, I raised the issue SciTools/iris#3019 to capture the oddity of a documentation example graphical test that is raising a UserWarning from within mpl...
<snip>/lib/python2.7/site-packages/matplotlib/ticker.py:1853: UserWarning: Steps argument should be a sequence of numbers
increasing from 1 to 10, inclusive. Behavior with
values outside this range is undefined, and will
raise a ValueError in future versions of mpl.
After digging a little, it turns out that the plot in question
uses nc-time-axis to plot time on the xaxis... and the weird step values that are being passed into the matplotlib.ticker.MaxNLocator that cause it to raise the UserWarning are [1, 2, 4, 7, 14] no less!
Awesome! So nc-time-axis is indeed the root cause.
I'll play with the steps to see the impact on plots...
There was a problem hiding this comment.
It took me a while to figure that out just to find out that @QuLogic already solved this last year 😄
| def test_yearly(self): | ||
| np.testing.assert_array_equal( | ||
| self.check(5, 0, 5*365), [31., 638., 1246., 1856.]) | ||
| self.check(5, 0, 5*365), [31., 485., 942., 1399., 1856.]) |
There was a problem hiding this comment.
With the change in the steps, and taking advantage of the classic style, the only test that fails is this one. I adapted the expected values to reflect the change but I'm not sure this is the ideal way to fix this. The README example gets 4 ticks instead of 3 with these changes but the figure looks good to me.
|
|
||
| if sys.version_info[:2] == (2, 7): | ||
| unittest.TestCase.assertRaisesRegex = unittest.TestCase.assertRaisesRegexp | ||
|
|
There was a problem hiding this comment.
@ocefpaf Could we not just use from six import assertRaisesRegex instead?
We'd need to add it as an dependency...
There was a problem hiding this comment.
Sure. Let me update that.
| val = [CalendarDateTime(cftime.datetime(2014, 8, 12), calendar_1), | ||
| CalendarDateTime(cftime.datetime(2014, 8, 13), calendar_2)] | ||
| with self.assertRaisesRegexp(ValueError, 'not all equal'): | ||
| with self.assertRaisesRegex(ValueError, 'not all equal'): |
There was a problem hiding this comment.
If you agree to using six then this will be
with assertRaisesRegex(self, ValueError, 'not all equal') :etc
setup.py
Outdated
| url='https://github.com/scitools/nc-time-axis', | ||
| packages=packages, | ||
| install_requires = ['matplotlib==1.*', | ||
| install_requires = ['matplotlib', |
There was a problem hiding this comment.
Not your doing, but let's put these in alphabetical order, thanks.
|
Fantastic! Thanks @ocefpaf. I'll cut a release with this in it shortly. |
4 participants
Closes #23